feat: all in one migration command #3250
Conversation
|
🚀 Deployed on https://deploy-preview-3250--moor.netlify.app |
There was a problem hiding this comment.
I was worried about supporting multiple databases, but splitting generated migration assets into different directories is a clever solution.
The implementation looks good to me, some things I'm wondering from a DX perspective:
- Most users will have exactly one database - I think we should make that case a little easier by also allowing a simple
String(as a file path) in thedatabasesoption (or perhaps with a separatedatabasekey). - Maybe we could use the entries of that map to not only configure the name of the database (which doesn't appear to be considered for file paths) but also the location under
test/where the default test should be added to? - I kind of view this as a "make the first migration tooling setup easier" command. In particular, I imagine that users will want to use the output of
writeGeneratedTest()as a starting point to add own migration tests too. So maybe we shouldn't overwrite the file every time and instead only write it if it doesn't exist?
|
1 will require fighting with freezed. Not sure how to do that? sealed unions? |
We're using json serializable. I think you can keep the field for this and then use |
|
We must resolve #3219 (comment) |
dickermoshe
force-pushed
the
drift_migrator_tool
branch
2 times, most recently
from
65ef5b3
to
3a37d49
Compare
|
@simolus3 How do you suppose I write tests for all this. Most of this functionality is copied from |
|
@simolus3 I did not implement point 1 myself. Left for you to implement - it's likely quicker for you given your familiarity with json_serializable. Regarding point 3, we could put a function in the generated Overall, I this is coming together much quicker than I would like. Please try this out yourself in practice and Lmk what the user experience is like. |
There was a problem hiding this comment.
This generally looks good to me. Some bigger notes are:
- We should adopt this in the
example/migrationsproject (if necessary, copy the existing schema dumps into the directory where this tool would expect them). Then let's add a section at the top of the readme showing how to use the new all-in-one command. We can keep the older commands as a reference for users who need manual control. - Since we expect that most users will want to run this command instead of the previous multiple invocations, let's highlight it more prominently. I think it deserves to be at the top of the migrations tooling page, mentioning that it does everything the other commands are doing in a single step, and that the other commands are then mostly intended for users needing more customization / integration into other tools.
- Instead of pointing to
--helpon the documentation website, I think the website should give an introduction into the command or at least point to the migrations example. The colorful usage output looks great, but users will probably expect this information on the site as well.
|
Agree, moved around some around of the docs. |
simolus3
force-pushed
the
drift_migrator_tool
branch
from
d06bc10
to
1d742e0
Compare
Generates Dumped Schema, Tests and Steps with a single command.
Uses
build.yamlconfig to define database and file paths.Fixes: #3216
This basically wraps
generate,dump andsteps`.A small refactor is needed to make the code in each of those commands reusable by this new command.